You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I implemented a new slicing backend (similar to how there are multiple backend for the projection) which uses nearest neighbor interpolation of the quantities as this is more suitable for moving mesh hydrodynamics.
All tests pass and I modified the relevant ones to also test the new backend. All features of the original slicing method (now still the default and dubbed "sph") should be supported by the new "nearest_neighbors" backend. No default behaviour has changed, so this should be fully backwards compatible.
I also added a few sentences to the documentation.
Do you think this is suitable for including in swiftsimIO?
Do you have any remarks/wishes?
Amazing! Love it. Thank you for this contribution; we should definitely include this. Thank you also for so carefully working within the existing framework.
It turns out np.cbrt is actually not supported by unyt_arrays (and hence cosmo_arrays). I opened a pull request to add support for it in unyt and changed the get_hsml implementation in the meantime.
Not required for this PR, but you could imagine actually not even needing a tree for this. For densely populated areas, you can just find the particle nearest to the pixel center by looping through all the particles, and for the sparsely populated areas a flood fill after that first procedure would work...
Not required for this PR, but you could imagine actually not even needing a tree for this. For densely populated areas, you can just find the particle nearest to the pixel center by looping through all the particles, and for the sparsely populated areas a flood fill after that first procedure would work...
Yeah, I did consider that actually, as it will probably be faster, but ended up using the tree method, since I already used it outside swiftwimio and hence was very easy to implement. Something to keep in mind for later probably though.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I implemented a new slicing backend (similar to how there are multiple backend for the projection) which uses nearest neighbor interpolation of the quantities as this is more suitable for moving mesh hydrodynamics.
All tests pass and I modified the relevant ones to also test the new backend. All features of the original slicing method (now still the default and dubbed
"sph") should be supported by the new"nearest_neighbors"backend. No default behaviour has changed, so this should be fully backwards compatible.I also added a few sentences to the documentation.
Do you think this is suitable for including in
swiftsimIO?Do you have any remarks/wishes?